Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: extract fetching the schema #33

Closed
wants to merge 4 commits into from

Conversation

mathieutu
Copy link
Contributor

Hi,
I've extracted an getSchema methods, which allows to override it to get several scheme, merge them, or custom the way we get it generally.

Thanks.

@wecc
Copy link
Member

wecc commented Nov 13, 2020

Thanks for you PR @mathieutu!

I took the liberty of renaming the method schema() (to better match the existing ones), update the CHANGELOG and add a test. I'm looking forward to getting this merged soon!

@wecc wecc changed the title refacto: extract getSchema method. feat: extract fetching the schema Nov 13, 2020
@wecc
Copy link
Member

wecc commented Nov 13, 2020

@emil-nasso @ttrig can you please have a look at this?

@mathieutu
Copy link
Contributor Author

Actually now I'm playing with extending/replacing the whole shema, maybe we can split that in a sdlSchemaString or something like that, and another true schema method which return the schema object.

	public function schema(): \GraphQL\Type\Schema
    {
        return BuildSchema::build($this->getSDLSchema(), [$this, 'decorateTypeConfig']);
    }

@wecc
Copy link
Member

wecc commented Nov 19, 2020

@mathieutu I'm thinking extendSchema(\GraphQL\Type\Schema): \GraphQL\Type\Schema, so you don't need to build everything (including resolving the schema content and appending decorateTypeConfig) from scratch. The default implementation would be an no-op just returning the provided schema.

@wecc
Copy link
Member

wecc commented Dec 10, 2020

Conflicts fixed and commits squashed in #40

@wecc wecc closed this in #40 Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants